-
Notifications
You must be signed in to change notification settings - Fork 158
New Plugin (Full Pull Request) #463
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Just an FYI for future reference: you don't need to create new PRs for new changes on GitHub. If you push to the same branch it will update the PR to include the changes on that branch. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if it matters for this PR (it can always be changed later) but AVIF is generally expected for thumbnails. You should probably either convert it on your own machine or look for an online converter to convert it for you, then upload it with the same basename but with .avif for the extension; also remember to update extensions.js to point to the AVIF version instead of the PNG.
| const date = new Date(args.DATE); | ||
| const timestamp = date.getTime(); | ||
|
|
||
| if (isNaN(timestamp)) return "Invalid Date"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there's an issue that cannot be continued from, you should throw, not return. Throw will render an error message on the GUI and highlight the script that had an issue, returning doesn't.
| // Create an 8-byte buffer | ||
| const buffer = new ArrayBuffer(8); | ||
| const view = new DataView(buffer); | ||
|
|
||
| // Write as Big-Endian 64-bit integer | ||
| // Note: BigInt is used because JS numbers lose precision above 53 bits | ||
| view.setBigUint64(0, BigInt(timestamp), false); | ||
|
|
||
| // Convert buffer to Hex string | ||
| return Array.from(new Uint8Array(buffer)) | ||
| .map(b => b.toString(16).padStart(2, '0')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a lot of work for something that is seemingly replicated perfectly fine with .toString(16).padStart(16, '0').toUpperCase(). Although feel free to correct me if I'm wrong, here.
| const hex = args.HEX.replace(/\s/g, ''); | ||
| const bytes = new Uint8Array(hex.match(/.{1,2}/g).map(byte => parseInt(byte, 16))); | ||
| const view = new DataView(bytes.buffer); | ||
|
|
||
| // Read as Big-Endian 64-bit integer | ||
| const timestamp = Number(view.getBigUint64(0, false)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yet again, I think this entire area could fairly well replaced with a singular call to parseInt, with the second parameter set to 16, to make sure it knows to parse as a 16 bit integer.
| const timestamp = Number(view.getBigUint64(0, false)); | ||
| return new Date(timestamp).toString(); | ||
| } catch (e) { | ||
| return "Invalid Hex"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once more, throw here. I really dislike seeing areas where random bugs can occur for no reason without even being able to track down where it's going on.
| { | ||
| name: "Date to Hex", | ||
| description: "Blocks to change a date to hex and vice versa.", | ||
| code: "AeroHutch/date-to-hex.js", | ||
| banner: "AeroHutch/date-to-hex.png", | ||
| creator: "AeroHutch", | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd put this at least a little further down in the list. Maybe next to format numbers and date format.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please be consistent about your usage of single quotes (') or double quotes (") for strings in this file. There really isn't a difference between the two (not in JavaScript, at least), you can easily interchange them. Given you use single quotes most often, I'd replace the double quotes in this file with single quotes.
|
|
||
| if (isNaN(timestamp)) return "Invalid Date"; | ||
|
|
||
| // Create an 8-byte buffer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment is pointless; comments should be used exceedingly sparingly, generally.
| // Write as Big-Endian 64-bit integer | ||
| // Note: BigInt is used because JS numbers lose precision above 53 bits | ||
| view.setBigUint64(0, BigInt(timestamp), false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Big endian is the default mode of this method, anyway. Specifying it to be written in big endian in a bit pointless: DataView.prototype.setBigUint64() littleEndian.
No description provided.